-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add beta-switch package #1263
base: master
Are you sure you want to change the base?
Add beta-switch package #1263
Conversation
const desireCookie = new RegExp("betaDesire=([^;]+);").exec( | ||
document.cookie[-1] === ";" ? document.cookie : document.cookie + ";" | ||
)?.[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaScript does not support negative array indexing. To check if the last character is a semicolon, replace document.cookie[-1]
with document.cookie[document.cookie.length - 1]
. This will properly access the last character of the string.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
enabledPages.forEach((regPath) => { | ||
const regExecuted = new RegExp(regPath).exec(window.location.pathname); | ||
const found = regExecuted !== null && regExecuted.length < 2; | ||
if (!betaIsAvailable && found) { | ||
betaIsAvailable = true; | ||
} | ||
}, betaIsAvailable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The forEach
has a second argument betaIsAvailable
passed as thisArg
which appears unintentional and could lead to bugs. The thisArg
parameter is meant to set the value of this
inside the callback function, but isn't needed here. Consider removing the second argument to forEach
.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
"main": "dist/thunderstore-beta-switch.cjs.js", | ||
"module": "dist/thunderstore-beta-switch.esm.js", | ||
"types": "dist/thunderstore-beta-switch.cjs.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a mismatch between the output paths specified in package.json
and TypeScript's actual output. TypeScript's tsc
command will generate different file names than what's listed here. To resolve this, either:
- Add a bundler (like Rollup or esbuild) to generate the
.cjs.js
and.esm.js
files, or - Update these paths to match TypeScript's output format (typically
.js
and.d.ts
files)
This will ensure the built files are available at the locations the package declares.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
No description provided.